Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved HL commander - spiral & linear segment #470

Merged
merged 7 commits into from
Oct 4, 2024

Conversation

matejkarasek
Copy link
Contributor

@matejkarasek matejkarasek commented Sep 9, 2024

This PR extends the functionality of the HL commander by adding 1) a linear option to the GOTO command and 2) a spiral command.
More details in the corresponding firmware PR: bitcraze/crazyflie-firmware#1410

@ataffanel
Copy link
Member

It seems that this PR will make the lib incompatible with previous version of the firmware.

Since the lib is currently not checking for the protocol version and locking connection, it needs to stay compatible with the old message. Either by checking the version and sending the appropriate message (maybe with a warning to update the Crazyflie), or by adding the new message in a new function.

@matejkarasek
Copy link
Contributor Author

Thanks for the review.
I see, I followed #134 where take_off was replaced by take_off_2...
Will make a new function instead 👌

@matejkarasek
Copy link
Contributor Author

matejkarasek commented Sep 25, 2024

So I made a new function.
Checking for the protocol version on the firmware side may have been more elegant, however, I have not figured out how to do that...

@knmcguire
Copy link
Member

I've just approved the CI run as that hasn't been done yet, so please check if there are any errors there.

Also for the review we'll need to try this out ourselves just to make sure that everything still works, so we will try to do that before the next release.

Yes absolutely, that would be a nice feature. I'm not sure if we have a issue/ticket for the version checking yet. 🤔

@knmcguire
Copy link
Member

The CI has failed on some autopep8 checks. @gemenerik will take a look at your PRs for test flight so if he can he'll fix it or you can fix it as well.

You can also run the test locally on your computer with the toolbelt: https://www.bitcraze.io/documentation/repository/toolbelt/master/ and tb verify. It should work for windows as well but there are some ifs/buts for the instructions.

@gemenerik gemenerik removed their assignment Sep 30, 2024
@matejkarasek
Copy link
Contributor Author

Thanks, the failed checks should now be fixed.

I have tested this on the crazyflie with both the latest release as well as with the new firmware bitcraze/crazyflie-firmware#1410. All works as expected.

But indeed this should be tested by someone else before it gets pulled into master.

cflib/crazyflie/high_level_commander.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@matejkarasek matejkarasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All should be fixed as requested. Succesfully flight tested

@gemenerik gemenerik merged commit c87fc89 into bitcraze:master Oct 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants